Skip to content

Conversation

@h1431532403240
Copy link
Contributor

Summary

Add support for compiling sudachi on Apple embedded platforms (iOS, tvOS, watchOS, visionOS).

Changes

Apple embedded platforms do not support dynamic library loading due to security restrictions. This adds a #[cfg] attribute to return an empty string for make_system_specific_name() on these platforms, which prevents DSO plugin loading while still allowing bundled plugins (com.worksap.nlp.sudachi.*) to work normally.

Motivation

Enable sudachi.rs to be used in iOS/tvOS/watchOS/visionOS applications via Swift bindings (UniFFI).

Testing

  • cargo build -p sudachi --target aarch64-apple-ios compiles successfully
  • cargo build -p sudachi --target aarch64-apple-ios-sim compiles successfully
  • Tested in iOS app with real dictionary tokenization

…isionOS)

Apple embedded platforms do not support dynamic library loading due to
security restrictions. This adds a cfg attribute to return an empty
string for make_system_specific_name() on these platforms, which
prevents DSO plugin loading while still allowing bundled plugins
(com.worksap.nlp.sudachi.*) to work normally.

Tested with:
- cargo build -p sudachi --target aarch64-apple-ios
- cargo build -p sudachi --target aarch64-apple-ios-sim
@h1431532403240 h1431532403240 marked this pull request as ready for review December 25, 2025 09:33
Copilot AI review requested due to automatic review settings December 25, 2025 09:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for Apple embedded platforms (iOS, tvOS, watchOS, visionOS) by preventing dynamic library loading on these platforms where it's not supported due to security restrictions. The change allows sudachi.rs to be used in mobile and embedded Apple applications while maintaining support for bundled plugins.

Key changes:

  • Added platform-specific implementation of make_system_specific_name() that returns an empty string for Apple embedded platforms

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

h1431532403240 added a commit to h1431532403240/sudachi-swift that referenced this pull request Dec 25, 2025
- UniFFI-based Swift bindings for sudachi.rs morphological analyzer
- Support for iOS 13+ and macOS 10.15+ (stable)
- Nightly builds for tvOS, watchOS, visionOS
- Bundled resources (char.def, unk.def, sudachi.json)
- Examples: BasicUsage (macOS) and iOSApp (iOS)
- CI/CD: build.yml for testing, release.yml for publishing
- CocoaPods and Swift Package Manager support

Note: Using fork of sudachi.rs with iOS platform support
PR: WorksApplications/sudachi.rs#308
@hayashi-mas-wap hayashi-mas-wap self-requested a review January 5, 2026 05:57
Copy link
Contributor

@hayashi-mas-wap hayashi-mas-wap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR. There are no issues with the functionality, but I have some requests regarding code improvements.

}

#[cfg(any(target_os = "ios", target_os = "tvos", target_os = "watchos", target_os = "visionos"))]
fn make_system_specific_name(_s: &str) -> String {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will let fn system_specific_name return a path to a directory and it may cause a problem.
I want you to change the signature of make_system_specific_name to fn (&str) -> Option<String> and let the variable fname (in l.80) to be None in those target OSs.

h1431532403240 and others added 2 commits January 5, 2026 21:42
- Change signature from fn(&str) -> String to fn(&str) -> Option<String>
- Return None for Apple embedded platforms instead of empty string
- Use and_then instead of map in system_specific_name for proper Option chaining

Addresses review feedback from @hayashi-mas-wap
@h1431532403240
Copy link
Contributor Author

Thank you for the review feedback! I've updated the implementation as suggested:

  • Changed make_system_specific_name signature from fn (&str) -> String to fn (&str) -> Option<String>
  • Apple embedded platforms now return None instead of an empty string
  • Updated system_specific_name to use .and_then(make_system_specific_name) instead of .map(), so fname becomes None on those target OSs
  • Updated the comment to reflect the new behavior

This ensures that system_specific_name properly returns None on iOS/tvOS/watchOS/visionOS, avoiding any potential issues with returning a path to a directory.

Copy link
Contributor

@hayashi-mas-wap hayashi-mas-wap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!!

@h1431532403240
Copy link
Contributor Author

LGTM, thank you!!

@hayashi-mas-wap
I noticed some formatting issues in the code during CI, and I’ve fixed them. Thanks for your assistance!

@hayashi-mas-wap hayashi-mas-wap merged commit 5e8cc92 into WorksApplications:develop Jan 7, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants